Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop shipping grafana-based image #7567

Merged
merged 13 commits into from
Jan 11, 2022
Merged

Stop shipping grafana-based image #7567

merged 13 commits into from
Jan 11, 2022

Conversation

alpeb
Copy link
Member

@alpeb alpeb commented Jan 6, 2022

Fixes #6045 #7358

With this change we stop building a Grafana-based image preloaded with the Linkerd Grafana dashboards.

Instead, we'll recommend users to install Grafana by themselves, and we provide a file grafana/values.yaml with a default config that points to all the same Grafana dashboards we had, which are now hosted in https://grafana.com/orgs/linkerd/dashboards .

The new file grafana/README.md contains instructions for installing the official Grafana Helm chart, and mentions other available methods.

The grafana.enabled flag has been removed, and grafanaUrl has been moved to grafana.url. This will help consolidating other grafana settings that might emerge, in particular when #7429 gets addressed.

Dashboards definitions changes

The dashboard definitions under grafana/dashboards (which should be kept in sync with what's published in https://grafana.com/orgs/linkerd/dashboards), got updated, adding the __inputs, __elements and __requires entries at the beginning, that were required in order to be published.

Labels additions

Added the linkerd.io/extension: viz label to some resources that were missing it. That way we can safely use linkerd viz install --prune -l linkerd.io/extension=viz in tests/integration/install_test.go, required for the grafana resources to be removed in the upgrade-edge|stable tests.

Fixes #6045 #7358

With this change we stop building a Grafana-based image preloaded with the Linkerd Grafana dashboards.

Instead, we'll recommend users to install Grafana by themselves, and we provide a file `grafana/values.yaml` with a default config that points to all the same Grafana dashboards we had, which are now hosted in https://grafana.com/orgs/linkerd/dashboards .

The new file `grafana/README.md` contains instructions for installing the official Grafana Helm chart, and mentions other available methods.

The `grafana.enabled` flag has been removed, and `grafanaUrl` has been moved to `grafana.url`. This will help consolidating other grafana settings that might emerge, in particular when #7429 gets addressed.

## Dashboards definitions changes

The dashboard definitions under `grafana/dashboards` (which should be kept in sync with what's published in https://grafana.com/orgs/linkerd/dashboards), got updated, adding the `__inputs`, `__elements` and `__requires` entries at the beginning, that were required in order to be published.
@alpeb alpeb requested a review from a team as a code owner January 6, 2022 14:58
Copy link
Member

@adleong adleong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the grafana README include instructions for how to publish dashboard changes to grafana.com?

Additionally, it would be nice to have some more detailed step-by-step instructions for installing grafana, and configuring linkerd-viz to use it but that probably belongs as a doc on the website.

@alpeb
Copy link
Member Author

alpeb commented Jan 7, 2022

Should the grafana README include instructions for how to publish dashboard changes to grafana.com?

I've just added the location in grafana.com at the end of the README, where dashboards can be managed.

Additionally, it would be nice to have some more detailed step-by-step instructions for installing grafana, and configuring linkerd-viz to use it but that probably belongs as a doc on the website.

Indeed. I'll followup with doc updates.

Copy link
Contributor

@Pothulapati Pothulapati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks great, and happy that we have one less thing to worry about already!

My suggestion would be to have

        {{- if .Values.grafana.url }}
        - -grafana-addr={{.Values.grafana.url}}
        {{- end}}

in the web.yaml and have grafana.url set to be empty by default. Because with the default install, we can't be sure where their grafana would be, and it probably won't be at grafana.grafana and thus renderinng errors when users click the grafana icon in the dashboard. Instead we should not display grafana icon by default, and have it only when grafana.url is set explicitly by the user. We can also update the docs stating this. WDYT?

grafana/README.md Show resolved Hide resolved
grafana/README.md Show resolved Hide resolved
grafana/README.md Outdated Show resolved Hide resolved
grafana/README.md Outdated Show resolved Hide resolved
@alpeb
Copy link
Member Author

alpeb commented Jan 10, 2022

My suggestion would be to have

        {{- if .Values.grafana.url }}
        - -grafana-addr={{.Values.grafana.url}}
        {{- end}}

in the web.yaml and have grafana.url set to be empty by default. Because with the default install, we can't be sure where their grafana would be, and it probably won't be at grafana.grafana and thus renderinng errors when users click the grafana icon in the dashboard. Instead we should not display grafana icon by default, and have it only when grafana.url is set explicitly by the user. We can also update the docs stating this. WDYT?

Makes sense 👍

alpeb added a commit to linkerd/website that referenced this pull request Jan 10, 2022
Followup to linkerd/linkerd2#7567

This is branched off of `alpeb/2.12`.

- Upgraded 2.12 upgrade notes, to explain linkerd viz no longer installs
  a grafana instance
- Created tasks/grafana.md explaining how to install and configure
  grafana and hookup the linkerd grafana dashboards
Copy link
Contributor

@Pothulapati Pothulapati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🥳 🎆

viz/cmd/install_test.go Show resolved Hide resolved
@alpeb alpeb merged commit 67dfebb into main Jan 11, 2022
@alpeb alpeb deleted the alpeb/grafana-spinoff branch January 11, 2022 19:47
alpeb added a commit to linkerd/website that referenced this pull request Jan 24, 2022
* Docs for Grafana removal

Followup to linkerd/linkerd2#7567

This is branched off of `alpeb/2.12`.

- Upgraded 2.12 upgrade notes, to explain linkerd viz no longer installs
  a grafana instance
- Created tasks/grafana.md explaining how to install and configure
  grafana and hookup the linkerd grafana dashboards
- Add section for off-cluster Grafana
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adapt to Grafana's new licensing
3 participants